From d4d0329547152a620c349438afd4aafe73841f42 Mon Sep 17 00:00:00 2001 From: Andrew Garrett Date: Fri, 27 Mar 2009 05:59:42 +0000 Subject: [PATCH] Revert r48746 (API userrights). Breaks Special:GlobalGroupMembership by changing overridden methods to static methods, which cannot be overridden. Also reverts r48747, r48814, r48778, r48775 --- RELEASE-NOTES | 1 - includes/AutoLoader.php | 1 - includes/User.php | 108 --------------- includes/api/ApiMain.php | 1 - includes/api/ApiQueryRecentChanges.php | 11 +- includes/api/ApiQueryUsers.php | 51 +------ includes/api/ApiUserrights.php | 117 ---------------- includes/specials/SpecialUserrights.php | 170 +++++++++++++++++------- 8 files changed, 132 insertions(+), 328 deletions(-) delete mode 100644 includes/api/ApiUserrights.php diff --git a/RELEASE-NOTES b/RELEASE-NOTES index 94fe56cfb8..0a70d97a76 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -339,7 +339,6 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN when someone tries to use them * BREAKING CHANGE: action=purge requires write rights and, for anonymous users, a POST request -* (bug 15935) Added action=userrights to add/remove users to/from groups * (bug 18099) Using appendtext to edit a non-existent page causes an interface message to be included in the page text * Added uiprop=changeablegroups to meta=userinfo diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 8edefe54f5..9404024c8d 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -292,7 +292,6 @@ $wgAutoloadLocalClasses = array( 'ApiRollback' => 'includes/api/ApiRollback.php', 'ApiUnblock' => 'includes/api/ApiUnblock.php', 'ApiUndelete' => 'includes/api/ApiUndelete.php', - 'ApiUserrights' => 'includes/api/ApiUserrights.php', 'ApiWatch' => 'includes/api/ApiWatch.php', 'Services_JSON' => 'includes/api/ApiFormatJson_json.php', 'Services_JSON_Error' => 'includes/api/ApiFormatJson_json.php', diff --git a/includes/User.php b/includes/User.php index 4798294079..cc861ad450 100644 --- a/includes/User.php +++ b/includes/User.php @@ -3231,114 +3231,6 @@ class User { return $text; } } - - /** - * Returns an array of the groups that a particular group can add/remove. - * - * @param $group String: the group to check for whether it can add/remove - * @return Array array( 'add' => array( addablegroups ), - * 'remove' => array( removablegroups ), - * 'add-self' => array( addablegroups to self), - * 'remove-self' => array( removable groups from self) ) - */ - static function changeableByGroup( $group ) { - global $wgAddGroups, $wgRemoveGroups, $wgGroupsAddToSelf, $wgGroupsRemoveFromSelf; - - $groups = array( 'add' => array(), 'remove' => array(), 'add-self' => array(), 'remove-self' => array() ); - if( empty($wgAddGroups[$group]) ) { - // Don't add anything to $groups - } elseif( $wgAddGroups[$group] === true ) { - // You get everything - $groups['add'] = self::getAllGroups(); - } elseif( is_array($wgAddGroups[$group]) ) { - $groups['add'] = $wgAddGroups[$group]; - } - - // Same thing for remove - if( empty($wgRemoveGroups[$group]) ) { - } elseif($wgRemoveGroups[$group] === true ) { - $groups['remove'] = self::getAllGroups(); - } elseif( is_array($wgRemoveGroups[$group]) ) { - $groups['remove'] = $wgRemoveGroups[$group]; - } - - // Re-map numeric keys of AddToSelf/RemoveFromSelf to the 'user' key for backwards compatibility - if( empty($wgGroupsAddToSelf['user']) || $wgGroupsAddToSelf['user'] !== true ) { - foreach($wgGroupsAddToSelf as $key => $value) { - if( is_int($key) ) { - $wgGroupsAddToSelf['user'][] = $value; - } - } - } - - if( empty($wgGroupsRemoveFromSelf['user']) || $wgGroupsRemoveFromSelf['user'] !== true ) { - foreach($wgGroupsRemoveFromSelf as $key => $value) { - if( is_int($key) ) { - $wgGroupsRemoveFromSelf['user'][] = $value; - } - } - } - - // Now figure out what groups the user can add to him/herself - if( empty($wgGroupsAddToSelf[$group]) ) { - } elseif( $wgGroupsAddToSelf[$group] === true ) { - // No idea WHY this would be used, but it's there - $groups['add-self'] = User::getAllGroups(); - } elseif( is_array($wgGroupsAddToSelf[$group]) ) { - $groups['add-self'] = $wgGroupsAddToSelf[$group]; - } - - if( empty($wgGroupsRemoveFromSelf[$group]) ) { - } elseif( $wgGroupsRemoveFromSelf[$group] === true ) { - $groups['remove-self'] = User::getAllGroups(); - } elseif( is_array($wgGroupsRemoveFromSelf[$group]) ) { - $groups['remove-self'] = $wgGroupsRemoveFromSelf[$group]; - } - - return $groups; - } - - /** - * Returns an array of groups that this user can add and remove - * @return Array array( 'add' => array( addablegroups ), - * 'remove' => array( removablegroups ), - * 'add-self' => array( addablegroups to self), - * 'remove-self' => array( removable groups from self) ) - */ - function changeableGroups() { - if( $this->isAllowed( 'userrights' ) ) { - // This group gives the right to modify everything (reverse- - // compatibility with old "userrights lets you change - // everything") - // Using array_merge to make the groups reindexed - $all = array_merge( User::getAllGroups() ); - return array( - 'add' => $all, - 'remove' => $all, - 'add-self' => array(), - 'remove-self' => array() - ); - } - - // Okay, it's not so simple, we will have to go through the arrays - $groups = array( - 'add' => array(), - 'remove' => array(), - 'add-self' => array(), - 'remove-self' => array() ); - $addergroups = $this->getEffectiveGroups(); - - foreach ($addergroups as $addergroup) { - $groups = array_merge_recursive( - $groups, $this->changeableByGroup($addergroup) - ); - $groups['add'] = array_unique( $groups['add'] ); - $groups['remove'] = array_unique( $groups['remove'] ); - $groups['add-self'] = array_unique( $groups['add-self'] ); - $groups['remove-self'] = array_unique( $groups['remove-self'] ); - } - return $groups; - } /** * Increment the user's edit-count field. diff --git a/includes/api/ApiMain.php b/includes/api/ApiMain.php index 42aaacc8c0..3fa0d72f2b 100644 --- a/includes/api/ApiMain.php +++ b/includes/api/ApiMain.php @@ -80,7 +80,6 @@ class ApiMain extends ApiBase { 'watch' => 'ApiWatch', 'patrol' => 'ApiPatrol', 'import' => 'ApiImport', - 'userrights' => 'ApiUserrights', ); /** diff --git a/includes/api/ApiQueryRecentChanges.php b/includes/api/ApiQueryRecentChanges.php index 94b1feeb52..9dacb86c8e 100644 --- a/includes/api/ApiQueryRecentChanges.php +++ b/includes/api/ApiQueryRecentChanges.php @@ -43,13 +43,12 @@ class ApiQueryRecentChanges extends ApiQueryBase { private $fld_comment = false, $fld_user = false, $fld_flags = false, $fld_timestamp = false, $fld_title = false, $fld_ids = false, $fld_sizes = false; - /** - * Get an array mapping token names to their handler functions. - * The prototype for a token function is func($pageid, $title, $rc) - * it should return a token or false (permission denied) - * @return array(tokenname => function) - */ + protected function getTokenFunctions() { + // tokenname => function + // function prototype is func($pageid, $title, $rev) + // should return token or false + // Don't call the hooks twice if(isset($this->tokenFunctions)) return $this->tokenFunctions; diff --git a/includes/api/ApiQueryUsers.php b/includes/api/ApiQueryUsers.php index 9f51c75705..51fa44e368 100644 --- a/includes/api/ApiQueryUsers.php +++ b/includes/api/ApiQueryUsers.php @@ -39,36 +39,6 @@ if (!defined('MEDIAWIKI')) { public function __construct($query, $moduleName) { parent :: __construct($query, $moduleName, 'us'); } - - /** - * Get an array mapping token names to their handler functions. - * The prototype for a token function is func($user) - * it should return a token or false (permission denied) - * @return array(tokenname => function) - */ - protected function getTokenFunctions() { - // Don't call the hooks twice - if(isset($this->tokenFunctions)) - return $this->tokenFunctions; - - // If we're in JSON callback mode, no tokens can be obtained - if(!is_null($this->getMain()->getRequest()->getVal('callback'))) - return array(); - - $this->tokenFunctions = array( - 'userrights' => array( 'ApiQueryUsers', 'getUserrightsToken' ), - ); - wfRunHooks('APIQueryUsersTokens', array(&$this->tokenFunctions)); - return $this->tokenFunctions; - } - - public static function getUserrightsToken($user) - { - global $wgUser; - // Since the permissions check for userrights is non-trivial, - // don't bother with it here - return $wgUser->editToken($user->getName()); - } public function execute() { $params = $this->extractRequestParams(); @@ -145,18 +115,6 @@ if (!defined('MEDIAWIKI')) { } if(isset($this->prop['emailable']) && $user->canReceiveEmail()) $data[$name]['emailable'] = ''; - if(!is_null($params['token'])) - { - $tokenFunctions = $this->getTokenFunctions(); - foreach($params['token'] as $t) - { - $val = call_user_func($tokenFunctions[$t], $user); - if($val === false) - $this->setWarning("Action '$t' is not allowed for the current user"); - else - $data[$name][$t . 'token'] = $val; - } - } } } // Second pass: add result data to $retval @@ -195,11 +153,7 @@ if (!defined('MEDIAWIKI')) { ), 'users' => array( ApiBase :: PARAM_ISMULTI => true - ), - 'token' => array( - ApiBase :: PARAM_TYPE => array_keys($this->getTokenFunctions()), - ApiBase :: PARAM_ISMULTI => true - ), + ) ); } @@ -213,8 +167,7 @@ if (!defined('MEDIAWIKI')) { ' registration - adds the user\'s registration timestamp', ' emailable - tags if the user can and wants to receive e-mail through [[Special:Emailuser]]', ), - 'users' => 'A list of users to obtain the same information for', - 'token' => 'Which tokens to obtain for each user', + 'users' => 'A list of users to obtain the same information for' ); } diff --git a/includes/api/ApiUserrights.php b/includes/api/ApiUserrights.php deleted file mode 100644 index 9e6c9f16f3..0000000000 --- a/includes/api/ApiUserrights.php +++ /dev/null @@ -1,117 +0,0 @@ -.@home.nl - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. - * http://www.gnu.org/copyleft/gpl.html - */ - -if (!defined('MEDIAWIKI')) { - // Eclipse helper - will be ignored in production - require_once ("ApiBase.php"); -} - - -/** - * @ingroup API - */ -class ApiUserrights extends ApiBase { - - public function __construct($main, $action) { - parent :: __construct($main, $action); - } - - public function execute() { - global $wgUser; - $params = $this->extractRequestParams(); - if(is_null($params['user'])) - $this->dieUsageMsg(array('missingparam', 'user')); - $user = User::newFromName($params['user']); - if($user->isAnon()) - $this->dieUsageMsg(array('nosuchuser', $params['user'])); - if(is_null($params['token'])) - $this->dieUsageMsg(array('missingparam', 'token')); - if(!$wgUser->matchEditToken($params['token'], $user->getName())) - $this->dieUsageMsg(array('sessionfailure')); - - $r['user'] = $user->getName(); - list($r['added'], $r['removed']) = - UserrightsPage::doSaveUserGroups( - $user, (array)$params['add'], - (array)$params['remove'], $params['reason']); - - $this->getResult()->setIndexedTagName($r['added'], 'group'); - $this->getResult()->setIndexedTagName($r['removed'], 'group'); - $this->getResult()->addValue(null, $this->getModuleName(), $r); - } - - public function mustBePosted() { - return true; - } - - public function isWriteMode() { - return true; - } - - public function getAllowedParams() { - return array ( - 'user' => array( - ApiBase :: PARAM_TYPE => 'user' - ), - 'add' => array( - ApiBase :: PARAM_TYPE => User::getAllGroups(), - ApiBase :: PARAM_ISMULTI => true - ), - 'remove' => array( - ApiBase :: PARAM_TYPE => User::getAllGroups(), - ApiBase :: PARAM_ISMULTI => true - ), - 'token' => null, - 'reason' => array( - ApiBase :: PARAM_DFLT => '' - ) - ); - } - - public function getParamDescription() { - return array ( - 'user' => 'User name', - 'add' => 'Add the user to these groups', - 'remove' => 'Remove the user from these groups', - 'token' => 'A userrights token previously retrieved through list=users', - 'reason' => 'Reason for the change', - ); - } - - public function getDescription() { - return array( - 'Add/remove a user to/from groups', - ); - } - - protected function getExamples() { - return array ( - 'api.php?action=userrights&user=FooBot&add=bot&remove=sysop|bureaucrat&token=123ABC' - ); - } - - public function getVersion() { - return __CLASS__ . ': $Id$'; - } -} diff --git a/includes/specials/SpecialUserrights.php b/includes/specials/SpecialUserrights.php index 71a6b0b30d..5e8d42ad0f 100644 --- a/includes/specials/SpecialUserrights.php +++ b/includes/specials/SpecialUserrights.php @@ -149,46 +149,29 @@ class UserrightsPage extends SpecialPage { $removegroup[] = $group; } } - self::doSaveUserGroups( $user, $addgroup, $removegroup, $reason ); - } - - /** - * Save user groups changes in the database. - * - * @param $user User object - * @param $add Array of groups to add - * @param $remove Array of groups to remove - * @param $reason String: reason for group change - * @return Array: Tuple of added, then removed groups - */ - static function doSaveUserGroups( $user, $add, $remove, $reason = '' ) { - global $wgUser; // Validate input set... - $isself = ($user->getName() == $wgUser->getName()); - $groups = $user->getGroups(); - $changeable = $wgUser->changeableGroups(); - $addable = array_merge( $changeable['add'], $isself ? $changeable['add-self'] : array() ); - $removable = array_merge( $changeable['remove'], $isself ? $changeable['remove-self'] : array() ); + $changeable = $this->changeableGroups(); + $addable = array_merge( $changeable['add'], $this->isself ? $changeable['add-self'] : array() ); + $removable = array_merge( $changeable['remove'], $this->isself ? $changeable['remove-self'] : array() ); - $remove = array_unique( - array_intersect( (array)$remove, $removable, $groups ) ); - $add = array_unique( array_diff( - array_intersect( (array)$add, $addable ), - $groups ) ); + $removegroup = array_unique( + array_intersect( (array)$removegroup, $removable ) ); + $addgroup = array_unique( + array_intersect( (array)$addgroup, $addable ) ); $oldGroups = $user->getGroups(); $newGroups = $oldGroups; // remove then add groups - if( $remove ) { - $newGroups = array_diff($newGroups, $remove); - foreach( $remove as $group ) { + if( $removegroup ) { + $newGroups = array_diff($newGroups, $removegroup); + foreach( $removegroup as $group ) { $user->removeGroup( $group ); } } - if( $add ) { - $newGroups = array_merge($newGroups, $add); - foreach( $add as $group ) { + if( $addgroup ) { + $newGroups = array_merge($newGroups, $addgroup); + foreach( $addgroup as $group ) { $user->addGroup( $group ); } } @@ -199,27 +182,29 @@ class UserrightsPage extends SpecialPage { wfDebug( 'oldGroups: ' . print_r( $oldGroups, true ) ); wfDebug( 'newGroups: ' . print_r( $newGroups, true ) ); - wfRunHooks( 'UserRights', array( &$user, $add, $remove ) ); + if( $user instanceof User ) { + // hmmm + wfRunHooks( 'UserRights', array( &$user, $addgroup, $removegroup ) ); + } if( $newGroups != $oldGroups ) { - self::addLogEntry( $user, $oldGroups, $newGroups, $reason ); + $this->addLogEntry( $user, $oldGroups, $newGroups ); } - return array( $add, $remove ); } - /** * Add a rights log entry for an action. */ - static function addLogEntry( $user, $oldGroups, $newGroups, $reason ) { + function addLogEntry( $user, $oldGroups, $newGroups ) { + global $wgRequest; $log = new LogPage( 'rights' ); $log->addEntry( 'rights', $user->getUserPage(), - $reason, + $wgRequest->getText( 'user-reason' ), array( - self::makeGroupNameListForLog( $oldGroups ), - self::makeGroupNameListForLog( $newGroups ) + $this->makeGroupNameListForLog( $oldGroups ), + $this->makeGroupNameListForLog( $newGroups ) ) ); } @@ -308,7 +293,7 @@ class UserrightsPage extends SpecialPage { return $user; } - static function makeGroupNameList( $ids ) { + function makeGroupNameList( $ids ) { if( empty( $ids ) ) { return wfMsgForContent( 'rightsnone' ); } else { @@ -316,11 +301,11 @@ class UserrightsPage extends SpecialPage { } } - static function makeGroupNameListForLog( $ids ) { + function makeGroupNameListForLog( $ids ) { if( empty( $ids ) ) { return ''; } else { - return self::makeGroupNameList( $ids ); + return $this->makeGroupNameList( $ids ); } } @@ -526,16 +511,111 @@ class UserrightsPage extends SpecialPage { } /** - * Returns $wgUser->changeableGroups() + * Returns an array of the groups that the user can add/remove. * * @return Array array( 'add' => array( addablegroups ), 'remove' => array( removablegroups ) , 'add-self' => array( addablegroups to self), 'remove-self' => array( removable groups from self) ) */ function changeableGroups() { global $wgUser; - $groups = $wgUser->changeableGroups(); + + if( $wgUser->isAllowed( 'userrights' ) ) { + // This group gives the right to modify everything (reverse- + // compatibility with old "userrights lets you change + // everything") + // Using array_merge to make the groups reindexed + $all = array_merge( User::getAllGroups() ); + return array( + 'add' => $all, + 'remove' => $all, + 'add-self' => array(), + 'remove-self' => array() + ); + } + + // Okay, it's not so simple, we will have to go through the arrays + $groups = array( + 'add' => array(), + 'remove' => array(), + 'add-self' => array(), + 'remove-self' => array() ); + $addergroups = $wgUser->getEffectiveGroups(); + + foreach ($addergroups as $addergroup) { + $groups = array_merge_recursive( + $groups, $this->changeableByGroup($addergroup) + ); + $groups['add'] = array_unique( $groups['add'] ); + $groups['remove'] = array_unique( $groups['remove'] ); + $groups['add-self'] = array_unique( $groups['add-self'] ); + $groups['remove-self'] = array_unique( $groups['remove-self'] ); + } + // Run a hook because we can - wfRunHooks( 'UserrightsChangeableGroups', array( $this, - $wgUser, $wgUser->getEffectiveGroups(), &$groups ) ); + wfRunHooks( 'UserrightsChangeableGroups', array( $this, $wgUser, $addergroups, &$groups ) ); + + return $groups; + } + + /** + * Returns an array of the groups that a particular group can add/remove. + * + * @param $group String: the group to check for whether it can add/remove + * @return Array array( 'add' => array( addablegroups ), 'remove' => array( removablegroups ) , 'add-self' => array( addablegroups to self), 'remove-self' => array( removable groups from self) ) + */ + private function changeableByGroup( $group ) { + global $wgAddGroups, $wgRemoveGroups, $wgGroupsAddToSelf, $wgGroupsRemoveFromSelf; + + $groups = array( 'add' => array(), 'remove' => array(), 'add-self' => array(), 'remove-self' => array() ); + if( empty($wgAddGroups[$group]) ) { + // Don't add anything to $groups + } elseif( $wgAddGroups[$group] === true ) { + // You get everything + $groups['add'] = User::getAllGroups(); + } elseif( is_array($wgAddGroups[$group]) ) { + $groups['add'] = $wgAddGroups[$group]; + } + + // Same thing for remove + if( empty($wgRemoveGroups[$group]) ) { + } elseif($wgRemoveGroups[$group] === true ) { + $groups['remove'] = User::getAllGroups(); + } elseif( is_array($wgRemoveGroups[$group]) ) { + $groups['remove'] = $wgRemoveGroups[$group]; + } + + // Re-map numeric keys of AddToSelf/RemoveFromSelf to the 'user' key for backwards compatibility + if( empty($wgGroupsAddToSelf['user']) || $wgGroupsAddToSelf['user'] !== true ) { + foreach($wgGroupsAddToSelf as $key => $value) { + if( is_int($key) ) { + $wgGroupsAddToSelf['user'][] = $value; + } + } + } + + if( empty($wgGroupsRemoveFromSelf['user']) || $wgGroupsRemoveFromSelf['user'] !== true ) { + foreach($wgGroupsRemoveFromSelf as $key => $value) { + if( is_int($key) ) { + $wgGroupsRemoveFromSelf['user'][] = $value; + } + } + } + + // Now figure out what groups the user can add to him/herself + if( empty($wgGroupsAddToSelf[$group]) ) { + } elseif( $wgGroupsAddToSelf[$group] === true ) { + // No idea WHY this would be used, but it's there + $groups['add-self'] = User::getAllGroups(); + } elseif( is_array($wgGroupsAddToSelf[$group]) ) { + $groups['add-self'] = $wgGroupsAddToSelf[$group]; + } + + if( empty($wgGroupsRemoveFromSelf[$group]) ) { + } elseif( $wgGroupsRemoveFromSelf[$group] === true ) { + $groups['remove-self'] = User::getAllGroups(); + } elseif( is_array($wgGroupsRemoveFromSelf[$group]) ) { + $groups['remove-self'] = $wgGroupsRemoveFromSelf[$group]; + } + return $groups; } -- 2.20.1